Move fast paths around to gain a bit of new-solver perf#158249
Conversation
|
This tests a bunch of changes together. I've done diffs locally suggesting ~5% speedup across these 7 commits. Moving fast path checks to @bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
Move fast path checks to `evaluate_goal_raw`
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (3f4f44d): comparison URL. Overall result: ❌✅ regressions and improvements - please read:Benchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf. Next, please: If you can, justify the regressions found in this try perf run in writing along with @bors rollup=never Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary 0.6%, secondary 1.6%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (secondary 3.2%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeResults (primary 0.0%, secondary 0.1%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Bootstrap: 503.321s -> 505.38s (0.41%) |
c340366 to
987b1a0
Compare
|
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Move fast path checks to `evaluate_goal_raw`
987b1a0 to
bfe02ca
Compare
bfe02ca to
bc602b3
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (b2fad67): comparison URL. Overall result: ❌✅ regressions and improvements - please read:Benchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf. Next, please: If you can, justify the regressions found in this try perf run in writing along with @bors rollup=never Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary 4.0%, secondary 3.3%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (secondary -6.6%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeResults (secondary 0.0%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Bootstrap: 509.16s -> 507.92s (-0.24%) |
23b11ab to
1cb6d00
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
|
@bors r=lcnr rollup=never (perf) |
This comment has been minimized.
This comment has been minimized.
What is this?This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.Comparing d62fb1c (parent) -> a6050b7 (this PR) Test differencesNo test diffs found Test dashboardRun cargo run --manifest-path src/ci/citool/Cargo.toml -- \
test-dashboard a6050b7ea848e25289bf097c606e0611c58ef4f3 --output-dir test-dashboardAnd then open Job duration changes
How to interpret the job duration changes?Job durations can vary a lot, based on the actual runner instance |
|
Finished benchmarking commit (a6050b7): comparison URL. Overall result: ❌✅ regressions and improvements - please read:Our benchmarks found a performance regression caused by this PR. Next Steps:
@rustbot label: +perf-regression Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (secondary -1.9%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (secondary -1.8%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis perf run didn't have relevant results for this metric. Bootstrap: 489.85s -> 492.034s (0.45%) |
|
(Friendly suggestion for next time: don't put the big table of results in the PR description because it ends up in the merge commit message and looks ugly in |
Change in % compared to previous commit on 4 benchmarks. All measured locally on ragdoll (AMD RYZEN 3990X, x86_64). These benchmarks have been quite repeatable in the past, but don't exactly match perf runs. All benchmarks show change in instruction count
First change is compared to baseline
Commit ids may change on rebases, so these links may become outdated.
View all comments